Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Box::into_raw instead of mem::forget-in-disguise #763

Merged
merged 1 commit into from
Jan 7, 2022
Merged

Use Box::into_raw instead of mem::forget-in-disguise #763

merged 1 commit into from
Jan 7, 2022
+6 −10

Conversation

taiki-e
Copy link
Member

@taiki-e taiki-e commented Jan 7, 2022 •
edited
Loading

This fixes the following stacked borrows violation.

MIRIFLAGS="-Zmiri-tag-raw-pointers" cargo miri test -p crossbeam-queue
error: Undefined Behavior: trying to reborrow for Unique at alloc67500, but parent tag <180167> does not have an appropriate item in the borrow stack
   --> /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/slice/raw.rs:130:14
    |
130 |     unsafe { &mut *ptr::slice_from_raw_parts_mut(data, len) }
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ trying to reborrow for Unique at alloc67500, but parent tag <180167> does not have an appropriate item in the borrow stack
    |
    = help: this indicates a potential bug in the program: it performed an invalid operation, but the rules it violated are still experimental
    = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
            
    = note: inside `std::slice::from_raw_parts_mut::<crossbeam_queue::array_queue::Slot<i32>>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/slice/raw.rs:130:14
    = note: inside `<crossbeam_queue::ArrayQueue<i32> as std::ops::Drop>::drop` at /Users/taiki/projects/crossbeam/crossbeam-queue/src/array_queue.rs:424:23
    = note: inside `std::ptr::drop_in_place::<crossbeam_queue::ArrayQueue<i32>> - shim(Some(crossbeam_queue::ArrayQueue<i32>))` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:188:1
note: inside `capacity` at crossbeam-queue/tests/array_queue.rs:24:5
   --> crossbeam-queue/tests/array_queue.rs:24:5
    |
24  |     }
    |     ^
note: inside closure at crossbeam-queue/tests/array_queue.rs:20:1
   --> crossbeam-queue/tests/array_queue.rs:20:1
    |
19  |   #[test]
    |   ------- in this procedural macro expansion
20  | / fn capacity() {
21  | |     for i in 1..10 {
22  | |         let q = ArrayQueue::<i32>::new(i);
23  | |         assert_eq!(q.capacity(), i);
24  | |     }
25  | | }
    | |_^
    = note: inside `<[closure@crossbeam-queue/tests/array_queue.rs:20:1: 25:2] as std::ops::FnOnce<()>>::call_once - shim` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `test::__rust_begin_short_backtrace::<fn()>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:575:5
    = note: inside closure at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:566:30
    = note: inside `<[closure@test::run_test::{closure#1}] as std::ops::FnOnce<()>>::call_once - shim(vtable)` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send> as std::ops::FnOnce<()>>::call_once` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/alloc/src/boxed.rs:1854:9
    = note: inside `<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>> as std::ops::FnOnce<()>>::call_once` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/panic/unwind_safe.rs:271:9
    = note: inside `std::panicking::r#try::do_call::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<(), std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<std::panic::AssertUnwindSafe<std::boxed::Box<dyn std::ops::FnOnce() + std::marker::Send>>, ()>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `test::run_test_in_process` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:598:18
    = note: inside closure at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:492:39
    = note: inside `test::run_test::run_test_inner` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:530:13
    = note: inside `test::run_test` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:562:28
    = note: inside `test::run_tests::<[closure@test::run_tests_console::{closure#2}]>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:305:17
    = note: inside `test::run_tests_console` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/console.rs:290:5
    = note: inside `test::test_main` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:116:15
    = note: inside `test::test_main_static` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/test/src/lib.rs:135:5
    = note: inside `main`
    = note: inside `<fn() as std::ops::FnOnce<()>>::call_once - shim(fn())` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:227:5
    = note: inside `std::sys_common::backtrace::__rust_begin_short_backtrace::<fn(), ()>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/sys_common/backtrace.rs:123:18
    = note: inside closure at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:145:18
    = note: inside `std::ops::function::impls::<impl std::ops::FnOnce<()> for &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>::call_once` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/core/src/ops/function.rs:259:13
    = note: inside `std::panicking::r#try::do_call::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<i32, &dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<&dyn std::ops::Fn() -> i32 + std::marker::Sync + std::panic::RefUnwindSafe, i32>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside closure at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:128:48
    = note: inside `std::panicking::r#try::do_call::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:406:40
    = note: inside `std::panicking::r#try::<isize, [closure@std::rt::lang_start_internal::{closure#2}]>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panicking.rs:370:19
    = note: inside `std::panic::catch_unwind::<[closure@std::rt::lang_start_internal::{closure#2}], isize>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/panic.rs:133:14
    = note: inside `std::rt::lang_start_internal` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:128:20
    = note: inside `std::rt::lang_start::<()>` at /Users/taiki/.rustup/toolchains/nightly-x86_64-apple-darwin/lib/rustlib/src/rust/library/std/src/rt.rs:144:17
    = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

cc #578

@taiki-e
Copy link
Member Author

taiki-e commented Jan 7, 2022

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 7, 2022

Build succeeded:

@bors bors bot merged commit d65ac67 into master Jan 7, 2022
@bors bors bot deleted the forget branch January 7, 2022 13:20
@taiki-e taiki-e mentioned this pull request Jan 8, 2022
Merged
bors bot added a commit that referenced this pull request Jan 8, 2022
@bors @taiki-e
Merge #769
f9cec06 
769: Prepare for the next release r=taiki-e a=taiki-e

- crossbeam-channel 0.5.1 -> 0.5.2
  - Fix stacked borrows violations. (#763, #764)
- crossbeam-epoch 0.9.5 -> 0.9.6
  - Add `Atomic::fetch_update`. (#706)
- crossbeam-queue 0.3.2 -> 0.3.3
  - Fix stacked borrows violation in `ArrayQueue`. (#763)
- crossbeam-utils 0.8.5 -> 0.8.6
  - Re-add `AtomicCell<{i,u}64>::{fetch_add,fetch_sub,fetch_and,fetch_or,fetch_xor}` that were accidentally removed in 0.8.0 on targets that do not support `Atomic{I,U}64`. (#767)
  - Re-add `AtomicCell<{i,u}128>::{fetch_add,fetch_sub,fetch_and,fetch_or,fetch_xor}` that were accidentally removed in 0.8.0. (#767)


Co-authored-by: Taiki Endo <[email protected]>
@taiki-e taiki-e mentioned this pull request Jan 22, 2022
Merged
bors bot added a commit that referenced this pull request Jan 22, 2022
@bors @taiki-e
Merge #774
6465d5e 
774: Reduce unsafe code in array queue/bounded channel r=taiki-e a=taiki-e

Before #458, since destructors could be called on elements, the correct way was to convert the Vec to a pointer, save it, and then [deallocate it as a Vec with zero length](https://github.com/crossbeam-rs/crossbeam/blob/28ad2b7e015832b47db7e389dd9ebce3e94b3adb/crossbeam-channel/src/flavors/array.rs#L549-L552). However, after #458, destructors of the elements wrapped by MaybeUninit are not called, so there is no need to save the Vec/boxed slice as a pointer. This removes the unsafe code that has caused several (potential) unsoundnesses in the past (#500, #533, #763).


Co-authored-by: Taiki Endo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
crossbeam-channel crossbeam-queue
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@taiki-e